Fix topic inheritance for translated documents #6633
Conversation
This commit addresses issues with topic handling for translated documents: 1. Adds signal handler to automatically propagate topic changes from parent documents to their translations, ensuring database consistency 2. Improves query filtering in facets.py to include translated documents based on their parent's topics, providing correct results even before database synchronization occurs These changes ensure that topics are consistently applied across all translations of a document, both at the database level and in query results.
24760fc to
4fb3cb1
Compare
|
I've modified this to just fix the case where the translated document isn't being displayed in the right list. This no longer updates translated document topics based on parent, or listens for signals. We may want to consider a solution for translation topics that doesn't keep bad data in the database - possibly just setting new translation document topics to null when they are created. |
There was a problem hiding this comment.
Good catch! Two things:
- I think it would be nice to have a test for this, to ensure it resolves Updating the topic of a parent article is not reflected inside the old topic listing page sumo#2183.
- Recommended addition in the comment below.
kitsune/wiki/facets.py
Outdated
| @@ -112,7 +112,9 @@ def _documents_for(user, locale, topics=None, products=None): | |||
| if topics: | |||
| topic_ids = [t.id for t in topics] | |||
| # we need to filter against parent topics for localized articles | |||
| qs = qs.filter(Q(topics__in=topic_ids) | Q(parent__topics__in=topic_ids)) | |||
| qs = qs.filter( | |||
| Q(topics__in=topic_ids) | Q(parent__isnull=False, parent__topics__in=topic_ids) | |||
There was a problem hiding this comment.
I think you should also add the parent check to the first Q clause:
Q(parent__isnull=True, topics__in=topic_ids) | ...
kitsune/wiki/tests/test_facets.py
Outdated
| @@ -267,28 +268,28 @@ def test_documents_for(self): | |||
| docs = _documents_for( | |||
| self.anonymous, locale="de", topics=[self.general_d, self.bookmarks_d] | |||
| ) | |||
| self.assertEqual([d["id"] for d in docs], [self.doc1_localized.id]) | |||
| self.assertEqual({d["id"] for d in docs}, {self.doc1_localized.id}) | |||
There was a problem hiding this comment.
Why is this changing? I suppose due to a change in ordering where two or more docs in the result ranked the same in terms of order, and are swapping positions from run to run and causing flaky results? If so, you can tweak the display_order to ensure that those docs are ordered consistently.
Since _documents_for returns document dicts in a specific order, we should continue using lists instead of sets.
There was a problem hiding this comment.
Thanks - sorry, I thought these were just membership tests. I'll revise.
kitsune/wiki/tests/test_facets.py
Outdated
|
|
||
| with self.subTest("documents_for-general_bookmarks_sync_localized-user1"): | ||
| docs = _documents_for( | ||
| self.user1, locale="de", topics=[self.general_d, self.bookmarks_d] | ||
| ) | ||
| self.assertEqual([d["id"] for d in docs], [self.doc1_localized.id]) | ||
| self.assertEqual({d["id"] for d in docs}, {self.doc1_localized.id}) |
kitsune/wiki/tests/test_facets.py
Outdated
|
|
||
| with self.subTest("documents_for-general_bookmarks_sync_localized-user2"): | ||
| docs = _documents_for( | ||
| self.user2, locale="de", topics=[self.general_d, self.bookmarks_d] | ||
| ) | ||
| self.assertEqual( | ||
| [d["id"] for d in docs], [self.doc1_localized.id, self.doc8_localized.id] | ||
| {d["id"] for d in docs}, {self.doc1_localized.id, self.doc8_localized.id} |
kitsune/wiki/tests/test_facets.py
Outdated
| ) | ||
|
|
||
| with self.subTest("documents_for-general_bookmarks_sync_localized-staff"): | ||
| docs = _documents_for( | ||
| self.staff, locale="de", topics=[self.general_d, self.bookmarks_d] | ||
| ) | ||
| self.assertEqual( | ||
| [d["id"] for d in docs], [self.doc1_localized.id, self.doc8_localized.id] | ||
| {d["id"] for d in docs}, {self.doc1_localized.id, self.doc8_localized.id} |
This commit addresses issues with topic handling for translated documents:
Adds signal handler to automatically propagate topic changes from parent documents to their translations, ensuring database consistency
Improves query filtering in facets.py to include translated documents based on their parent's topics, providing correct results even before database synchronization occurs
These changes ensure that topics are consistently applied across all translations of a document, both at the database level and in query results.